I agree with the proposed direction of travel. The way I see it, there are two approaches to name overloading:

1. Type-driven name resolution
2. Class-based polymorphism

Haskell normally uses (2), but in rare cases ends up using (1), one such case being DuplicateRecordFields. We should aspire to use (2) always, and the HasField machinery is exactly that.

I feel a bit silly bringing this up, but might you rename the proposal something like

DuplicateRecordFields without ambiguous selectors field access"

Since you define "field selector occurrence" and "field update occurrence", and the proposal applies to both, I think it's confusing to title it with the word "selector".

I agree the "Type-driven name resolution" that's done is bad. I'm not sure "Class-based polymorphism" is good, but I remain a little hopeful the "Name-driven" resolution I went for in #310 becomes viable after all. I like this proposal because it just pushes us away from type-driven resolution, and doesn't bias one alternative over another.

Regarding type-directed name resolution, my feeling is that it is in principle a useful approach, particularly in languages that are heavily slanted towards bidirectional type-checking (e.g. Idris, Agda). That is, I think TDNR done right can work well. But the use in DuplicateRecordFields is clearly not "TDNR done right", and indeed the interactions with a wider context that prioritises class-based resolution mean that it is better to just drop the TDNR aspect from DuplicateRecordFields.

(A good bidirectional/TDNR story for record selectors really needs dot-notation, because then you can infer the type of the record being accessed and use that to disambiguate the selector. But with RecordDotSyntax taking the class-based approach I think that ship has sailed.)

I feel a bit silly bringing this up, but might you rename the proposal something like

DuplicateRecordFields without ambiguous selectors field access"

Since you define "field selector occurrence" and "field update occurrence", and the proposal applies to both, I think it's confusing to title it with the word "selector".

@Ericson2314 thanks, that's a good point. (I originally wrote the proposal for selectors, then decided to add updates, and of course I forgot to change the title. 🤦) I've updated the PR.

I'm strongly in favour of this proposal. The current design is an egregious hack; I don't think it would every have got past this committee! Moreover, it has a pretty significant implementation cost; that would be OK if it were in service of a principled goal, but it isn't.

Let's tidy this up.

I'm also in strong support of this proposal. Indeed, I would advocate for a shorter, more abrupt transition period. Maybe we warn for a release, but then just rip out the existing machinery. Any change to code is fully backward compatible.

Users could easily convince me that a slower transition is better, but I don't think we designers should assume that users need the slower transition.

I'm not formally a part of the approval process, but I did want to chime in with a strong +1 in favor of this proposal. DuplicateRecordFields is a horrible wart, and this seems to shave it down to a reasonable nub.

I would advocate for a shorter, more abrupt transition period. Maybe we warn for a release, but then just rip out the existing machinery.

@goldfirere the reaction to this proposal is making me agree we should rip it out ASAP. So default to -Wwarn=ambiguous-fields for a release, then gone in the next release? If everything goes perfectly we might aim to get RecordDotSyntax, NoFieldSelectors and -Wwarn=ambiguous-fields into 9.2, then drop the hack completely in 9.4?

(Incidentally, this gist from @chrisdone shows how the module system can be used nicely to avoid field occurrence ambiguity in modules other than the defining module. I half wonder if we should allow modules to import themselves qualified and with an explicit import list, so this trick would work in the defining module as well...)

Yes, I support the more rapid removal @adamgundry proposed.

this gist from @chrisdone shows how the module system can be used nicely to avoid field occurrence ambiguity in modules other than the defining module.

cute! Isn't there a more general feature lurking here, not specific to ambiguous record field names? (They merely bite the worst.) For example I've seen loose talk about multi-module modules: that is, one source file with multiple module decls, mutually importing each other.

There are for example StackOverflow questions asking why you can't use duplicate names for data constructors, especially for enumerated types: Orange as a colour vs as a fruit; Dec, Oct as a month vs as a number base. And I guess there's some generic method names (make, build, empty, singleton, size, length) that are likely to be ambiguous amongst classes.

I half wonder if we should allow modules to import themselves qualified and with an explicit import list, so this trick would work in the defining module as well

mmm that seems heavy machinery. Would something like this be more succinct (but using the same name prefixing under the covers)

data Person qualified = Person { name :: String, age :: Int } deriving Show
data Company qualified as Co = Company { name :: String, age :: Int } deriving Show

data Colour qualified = Red | Orange | Yellow | Green | Blue | Indigo | Violet

class Shape a qualified  where { area :: ...; perimeter :: ...; centre :: ...    }

in which if the as-part is omitted qualified as <name to my left> is assumed.

(hmm hmm I used qualified because it's already a reserved word, not likely to be used in existing code; whereas bare as is liable to clash with tyvar names. OTOH import qualified means the name must always be used qualified -- which is probably onerous.)

@AntC2 #283

@AntC2 yes, local modules would be another way to be able to disambiguate field occurrences, and there might indeed be some merit in having datatype declarations imply local modules. FWIW, being able to refer to the enclosing module with a different prefix and an explicit import list has come up as desirable before (https://gitlab.haskell.org/ghc/ghc/-/issues/15432). But in any case this is out of scope of the current proposal.

So far the only dissatisfaction I've seen with this proposal is a single 👎 from @klapaucius. Would you be willing to elaborate?

I've thrown together a rough implementation of the new warning at https://gitlab.haskell.org/ghc/ghc/-/commit/abf0fb3138bd05a94fe69fb887cf308e51d3a7d4 ... anyone fancy bikeshedding the warning text?

overloadedrecfldsfail06.hs:15:22: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
    The record update u {x = True} with type U is ambiguous.
    This will not be supported by -XDuplicateRecordFields in future releases of GHC.

overloadedrecfldsfail12.hs:13:5: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
    The field ‘foo’ belonging to type T is ambiguous.
    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
    You can use a qualified import or explicit case analysis to resolve the ambiguity.

overloadedrecfldsfail12.hs:16:5: error: [-Wambiguous-fields (in -Wdefault), -Werror=ambiguous-fields]
    The field ‘foo’ belonging to type S is ambiguous.
    This will not be supported by -XDuplicateRecordFields in future releases of GHC.
    You can use explicit case analysis to resolve the ambiguity.

@adamgundry

I prefer that existing features aren't removed unless they are really problematic. Such removals induce unnecessary anxiety about extensions and I'm not looking forward to discussions with my colleagues what we shouldn't use some extensions because "they can be removed at any moment just like TDNR in DuplicateRecordFields".
I agree that TDNR is somewhat problematic, but I believe that it shouldn't be fixed with an even more problematic extension.
If the breakage is necessary I would prefer to disambiguate selectors with a type or constructor name.
I believe that it would be better to wait until something like proposal 283 is accepted and implemented.

@nomeata I believe this proposal is ready for review. From the discussion:

• Several people expressed support for this change. The proposal originally suggested a possibly longer transition period, but this was amended in the light of feedback.
• @klapaucius raised concerns about the impact of breaking existing working features (see below).
• There was some discussion of possible extensions to the module system that might help with name ambiguity problems, but I don't think these need hold up this proposal and they can be discussed elsewhere.

@klapaucius thanks for the feedback. I do acknowledge that breaking changes are always painful, but if we don't make them we are stuck with dubious design decisions forever, and here I think the consensus is that this feature is one we could do without. I've added a line to the "Costs and Drawbacks" section, and added a new section documenting migration strategies for fixing existing code.

@nomeata ping; I would like to submit this proposal for committee review.

Thanks for the ping!

I'm in favour of this proposal for several reasons, all of which have already been given above. I think the current behaviour is unintuitive, and this proposal's explicit presentation of the rules (under which the current TDNR works) is the first time I've really thought about them beyond hand-waving or trial and error. I'm therefore going to recommend that we accept this, pending committee review.

Hey! We've discussed this as a committee, and we agree that it should go ahead. This proposal will now be marked as accepted, and @nomeata will merge it in for us. Thanks for submitting the proposal!

This proposal mentions RecordDotSyntax in its migration strategy section. But it seems that, since proposal 405, that proposed extension has become two extensions: OverloadedRecordSelection and OverloadedRecordUpdate. Could we edit the migration strategy section to take this into account? Otherwise it's quite confusing for people hitting -Wambiguous-fields to know what to do.